-
-
Notifications
You must be signed in to change notification settings - Fork 18k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
register custom DisplayFormatter for table schema #16198
Conversation
instead of using `_ipython_display_` for custom mime-types
from IPython.core.formatters import BaseFormatter | ||
|
||
class TableSchemaFormatter(BaseFormatter): | ||
print_method = '_repr_table_schema_' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm beginning to wonder if we should be calling this repr_data_resource, based on the mimetype we ended up at (which includes the schema inside)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The idea being "table schema" refers to just the schema? Makes sense to me.
About the UnserializeableWarning, what's the behavior from the PR when pandas For background, some DataFrames just can't be serialized according to the table schema spec. In these cases, we'd ideally fall back to |
Oh neat, it looks like it falls back to the next formatter. That's perfect. So we can just get rid of the |
You'll get a traceback (not user-mute-able like a warning) with the failure and other reprs will still display. If that's okay, I'll just remove the warning. |
|
||
expected = {'text/plain', 'text/html'} | ||
assert set(arg.keys()) == expected | ||
assert set(formatted[0].keys()) == expected | ||
assert "orient='table' is not supported for MultiIndex" in ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will have to remove this line.
with pd.option_context('display.html.table_schema', True): | ||
assert 'application/vnd.dataresource+json' in formatters | ||
assert formatters[mimetype].enabled | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think our linter complained about whitespace here.
pandas/core/generic.py
Outdated
reprs = {"text/plain": text, "text/html": html, "text/latex": latex, | ||
"application/vnd.dataresource+json": table_schema} | ||
reprs = {k: v for k, v in reprs.items() if v} | ||
display(reprs, raw=True) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some whitespace issues here https://travis-ci.org/pandas-dev/pandas/jobs/228015735#L1354
@minrk I did those minor fixes. Thanks for putting this together. Feels much better than monkey patching :) |
Codecov Report
@@ Coverage Diff @@
## master #16198 +/- ##
==========================================
- Coverage 90.86% 90.86% -0.01%
==========================================
Files 162 162
Lines 50871 50867 -4
==========================================
- Hits 46225 46221 -4
Misses 4646 4646
Continue to review full report at Codecov.
|
1 similar comment
Codecov Report
@@ Coverage Diff @@
## master #16198 +/- ##
==========================================
- Coverage 90.86% 90.86% -0.01%
==========================================
Files 162 162
Lines 50871 50867 -4
==========================================
- Hits 46225 46221 -4
Misses 4646 4646
Continue to review full report at Codecov.
|
Thanks Min! |
Thanks, @TomAugspurger! |
Version 0.20.0 * tag 'v0.20.0': (742 commits) RLS: v0.20.0 DOC: Whatsnew cleanup (pandas-dev#16245) TST: Test CategoricalIndex in test_is_categorical (pandas-dev#16243) TST: xfail some bottleneck on windows (pandas-dev#16240) DOC, TST: Document and Test Functions in dtypes/common.py (pandas-dev#16237) TST: Remove __init__ statements in testing (pandas-dev#16238) DOC: don't include all methods/attributes of IntervalIndex (pandas-dev#16221) PKG: Fix ModuleNotFoundError: No module named 'pandas.formats' (pandas-dev#16239) RLS: v0.20.0rc2 CLN: make submodules of pandas.util private (pandas-dev#16223) MAINT: Remove tm.TestCase from testing (pandas-dev#16225) MAINT: Complete Conversion to Pytest Idiom (pandas-dev#16201) DOC: add whatsnew for 0.21.0 DEPR: correct deprecation message for datetools (pandas-dev#16202) API Change repr name for table schema (pandas-dev#16204) DOC: Remove various warnings from doc build (pandas-dev#16206) DOC: add whatsnew for v0.20.1 BUG: Fixed renaming of falsey names in build_table_schema (pandas-dev#16205) COMPAT: ensure proper extension dtype's don't pickle the cache (pandas-dev#16207) REF: register custom DisplayFormatter for table schema (pandas-dev#16198) ...
instead of using
_ipython_display_
for custom mime-typesThis should solve the issues in #16171, without needing to register/unregister
_ipython_display_
.In the long run, ipython/ipython#10496 should remove the need for any custom registration, in favor of a simple
_repr_mimebundle_
method.I think the only thing missing is the UnserializableWarning because IPython does its own catching and displaying of tracebacks when formatters fail. I can copy and modify a little more code out of the base Formatter to preserve that warning, though.